-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add simple examples of gem usage #20
Conversation
WalkthroughThe recent changes encompass updates to the handling of parameters passed to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- README.md (4 hunks)
- examples/download_and_move_file.rb (1 hunks)
- examples/download_file.rb (1 hunks)
- examples/move_file.rb (1 hunks)
Additional context used
Markdownlint
README.md
198-198: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (7)
examples/move_file.rb (2)
32-37
: LGTM!The
move_file!
method is well-implemented. The use ofFileUtils.mv
is appropriate and the error handling forErrno::ENOENT
is correctly implemented.
10-30
: LGTM! Ensure task options passed toUberTask.run
are correct.The
run
method is well-structured and usesUberTask.run
effectively for task management. The success and retry handlers are implemented correctly.However, ensure that the task options passed to
UberTask.run
are correct and complete.Verification successful
Verified: Task options passed to
UberTask.run
are correct and complete.The
run
method in theMoveFile
class correctly passes the task options toUberTask.run
, includingdefault_retry_wait
,retry_count
,vital
, and**task_options
.
- File:
examples/move_file.rb
- Lines: 10-30
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify task options passed to `UberTask.run` in `MoveFile` class. # Test: Search for the `UberTask.run` usage in the `MoveFile` class. Expect: Correct task options are passed. rg --type ruby -A 5 $'class MoveFile\n def self.run'Length of output: 245
Script:
#!/bin/bash # Description: Verify task options passed to `UberTask.run` in `MoveFile` class. # Test: Search for the `UberTask.run` usage in the `MoveFile` class with multiline mode enabled. Expect: Correct task options are passed. rg --multiline --type ruby -A 5 $'class MoveFile\n def self.run'Length of output: 65
Script:
#!/bin/bash # Description: Verify the presence of the `MoveFile` class and its methods. # Step 1: Search for the `MoveFile` class definition. rg --type ruby 'class MoveFile' # Step 2: If the class is found, search for the `run` method within the `MoveFile` class. rg --type ruby 'def self.run' --context 5Length of output: 2439
Script:
#!/bin/bash # Description: Verify the task options passed to `UberTask.run` in the `MoveFile` class by extracting the full context of the `run` method. # Extract the full context of the `run` method within the `MoveFile` class. rg --type ruby -A 20 'def self.run' examples/move_file.rbLength of output: 610
examples/download_and_move_file.rb (1)
11-42
: LGTM! Ensure task options passed toUberTask.run
are correct.The
run
method is well-structured and usesUberTask.run
effectively for task management. The success and subtask error handlers are implemented correctly. The calls toDownloadFile.run
andMoveFile.run
are appropriate.However, ensure that the task options passed to
UberTask.run
are correct and complete.Verification successful
Task options for
UberTask.run
are correct.The
run
method inDownloadAndMoveFile
correctly passes the task name and retry count options toUberTask.run
. The snippet shows the task name 'Download and move file' andretry_count: 2
, which are appropriate.
examples/download_and_move_file.rb: UberTask.run('Download and move file', retry_count: 2)
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify task options passed to `UberTask.run` in `DownloadAndMoveFile` class. # Test: Search for the `UberTask.run` usage in the `DownloadAndMoveFile` class. Expect: Correct task options are passed. rg --type ruby -A 5 $'class DownloadAndMoveFile\n def self.run'Length of output: 256
Script:
#!/bin/bash # Description: Verify task options passed to `UberTask.run` in `DownloadAndMoveFile` class. # Test: Search for the `UberTask.run` usage in the `DownloadAndMoveFile` class in multiline mode. Expect: Correct task options are passed. rg --type ruby -U -A 5 $'class DownloadAndMoveFile\n def self.run'Length of output: 67
Script:
#!/bin/bash # Description: Broaden the search to locate `DownloadAndMoveFile` class and `UberTask.run` calls. # Step 1: Search for the `DownloadAndMoveFile` class definition. rg --type ruby 'class DownloadAndMoveFile' # Step 2: Search for `UberTask.run` calls in the codebase to verify the task options passed. rg --type ruby 'UberTask.run'Length of output: 615
Script:
#!/bin/bash # Description: Extract and review the `UberTask.run` call in `examples/download_and_move_file.rb` to verify the task options passed. # Extract the relevant `UberTask.run` call in `examples/download_and_move_file.rb`. rg --type ruby -A 10 'UberTask.run' examples/download_and_move_file.rbLength of output: 479
examples/download_file.rb (2)
37-42
: LGTM!The
save_external_file!
method is well-implemented. The use ofURI.parse(source).open
andFile.write
is appropriate. The error handling forOpenURI::HTTPError
is correctly implemented.
13-35
: LGTM! Ensure task options passed toUberTask.run
are correct.The
run
method is well-structured and usesUberTask.run
effectively for task management. The retry and success handlers are implemented correctly. The call tosave_external_file!
is appropriate.However, ensure that the task options passed to
UberTask.run
are correct and complete.Verification successful
The task options passed to
UberTask.run
are correct and complete.The
examples/download_file.rb
file includes theDownloadFile
class and therun
method. The task optionsdefault_retry_wait
,retry_count
,vital
, and additionaltask_options
are appropriately passed toUberTask.run
. The implementation is well-structured.
- The
DownloadFile
class andrun
method are correctly defined.- Task options passed to
UberTask.run
are appropriate and comprehensive.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify task options passed to `UberTask.run` in `DownloadFile` class. # Test: Search for the `UberTask.run` usage in the `DownloadFile` class. Expect: Correct task options are passed. rg --type ruby -A 5 $'class DownloadFile\n def self.run'Length of output: 249
Script:
#!/bin/bash # Description: Verify task options passed to `UberTask.run` in `DownloadFile` class. # Test: Search for the `UberTask.run` usage in the `DownloadFile` class. Expect: Correct task options are passed. rg --type ruby --multiline -A 5 $'class DownloadFile\n def self.run'Length of output: 69
Script:
#!/bin/bash # Description: Verify task options passed to `UberTask.run` in `DownloadFile` class. # Locate all Ruby files and search for the `DownloadFile` class and `run` method within those files. fd --type f --extension rb --exec rg --multiline -A 5 $'class DownloadFile\n def self.run' {}Length of output: 96
Script:
#!/bin/bash # Description: Verify task options passed to `UberTask.run` in the `DownloadFile` class. # Check the contents of the examples/download_file.rb file for the `DownloadFile` class and the `run` method. if [ -f examples/download_file.rb ]; then cat examples/download_file.rb | rg --multiline -A 5 $'class DownloadFile\n def self.run' else echo "File examples/download_file.rb not found." fiLength of output: 128
Script:
#!/bin/bash # Description: Verify task options passed to `UberTask.run` in the `DownloadFile` class. # Print the contents of the examples/download_file.rb file to manually verify the presence of the `DownloadFile` class and the `run` method. cat examples/download_file.rbLength of output: 2339
README.md (2)
55-55
: LGTM!The change to pass the
name
parameter as a positional argument in theUberTask.run
method is correct.
110-110
: LGTM!The change to pass the
name
parameter as a positional argument in theUberTask.run
method is correct.
examples/download_and_move_file.rb
Outdated
'and an early return can be made, ' \ | ||
'isn\'t that cool!?'.underline, | ||
) | ||
return # Try to remove this line and see what happens! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the return, returns inside this block will affect the method that handles the exection.
We have to raise RetryTask, SkipTask or EventHandled, and if we don't raise either of these options, the exception will continue to bubble up.
It looks like I forgot to add a UberTask.event_handled method, add that method. All events have a .handled method already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the return and added handling different types of exceptions which can be raised by subtasks
examples/download_and_move_file.rb
Outdated
to: download_path, | ||
vital: true, | ||
) | ||
rescue StandardError => err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the error check inside on_subtask_error, which will show the main feature, centralized error reporting, and how the parent task can force a child task to skip or retry. Here's an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved error check to on_subtask_error
examples/download_file.rb
Outdated
|
||
def self.save_external_file!(source:, to:) | ||
URI.parse(source).open { |io| File.write(to, io.read) } | ||
rescue OpenURI::HTTPError => err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this check to the parent task on_subtask_error; all errors bubble up to the parent, like in Javascript and if the parent asks the task to be retried, the current task (child) will run again.
This was the main idea to deploy apps on popmenu, we have a single place for common network errors and tasks can have many subtasks but all of them have common network errors and each time they happen, the on_subtask_error of the top task checks the error type and just ask to retry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved rescue
from here to parent task's on_subtask_error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@borela Thanks for review! Applied your suggestions and simplified examples, can you take another look?
examples/download_and_move_file.rb
Outdated
'and an early return can be made, ' \ | ||
'isn\'t that cool!?'.underline, | ||
) | ||
return # Try to remove this line and see what happens! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the return and added handling different types of exceptions which can be raised by subtasks
examples/download_and_move_file.rb
Outdated
to: download_path, | ||
vital: true, | ||
) | ||
rescue StandardError => err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved error check to on_subtask_error
examples/download_file.rb
Outdated
|
||
def self.save_external_file!(source:, to:) | ||
URI.parse(source).open { |io| File.write(to, io.read) } | ||
rescue OpenURI::HTTPError => err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved rescue
from here to parent task's on_subtask_error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (4)
examples/failing_task.rb (2)
1-1
: Add a description comment for the file.Consider adding a file-level comment to describe the purpose of this script.
# This script defines a failing task example using UberTask.
6-7
: Add a class-level comment.Consider adding a comment to describe the purpose of the
FailingTask
class.# This class represents a task that intentionally fails to demonstrate error handling with UberTask.
README.md (2)
196-197
: Specify the language for the code block.To improve readability and adhere to Markdownlint guidelines, specify the language for the code block.
- ``` + ```rubyTools
Markdownlint
197-197: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
55-55
: Inconsistent usage ofUberTask.run
method signatureThe method
UberTask.run
is not consistently updated to use the new signature across the codebase. Please update the following instances to match the new signature:
examples/move_file.rb
examples/download_file.rb
spec/non_rails/uber_task/skip_spec.rb
spec/non_rails/uber_task/run_spec.rb
spec/non_rails/uber_task/exception_spec.rb
Make sure all usages of
UberTask.run
follow the new signature format to ensure consistency and correctness in the codebase.Analysis chain
Verify the method signature change.
Ensure that changing the
name
parameter to a positional argument inUberTask.run
is consistent throughout the codebase and documentation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `UberTask.run` match the new signature. # Test: Search for the method usage. Expect: Only occurrences of the new signature. rg --type ruby -A 5 $'UberTask.run'Length of output: 2897
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- README.md (4 hunks)
- examples/download_and_move_file.rb (1 hunks)
- examples/download_file.rb (1 hunks)
- examples/failing_task.rb (1 hunks)
- examples/move_file.rb (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- examples/download_and_move_file.rb
- examples/download_file.rb
- examples/move_file.rb
Additional context used
Markdownlint
README.md
197-197: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (2)
examples/failing_task.rb (1)
9-13
: Good use of UberTask and custom error handling.The
run
method correctly demonstrates the use ofUberTask.run
with custom error handling.README.md (1)
110-110
: Verify the method signature change.Ensure that changing the
name
parameter to a positional argument inUberTask.run
is consistent throughout the codebase and documentation.
What does this PR do?
This PR adds simple examples of gem usage
Screenshots
Summary by CodeRabbit
New Features
DownloadAndMoveFile
,DownloadFile
, andMoveFile
modules.FailingTask
class to handle error scenarios.Refactor
UberTask.run
method calls to pass thename
parameter as a positional argument instead of a named argument for improved consistency.